[cmake] move ExternalProject_Add to subdir for consistency, and define proper GSL,FFTW,TBB targets by ferdymercury · Pull Request #21904 · root-project/root

ferdymercury · 2026-04-13T13:17:50Z

Try unbloating the huge SearchInstalledSoftware
and use always subdirs for ExternalProject_Add for symmetry with other builtins

github-actions · 2026-04-13T16:02:06Z

Test Results

22 files 22 suites 3d 4h 49m 28s ⏱️
3 833 tests 3 831 ✅ 1 💤 1 ❌
75 650 runs 75 631 ✅ 18 💤 1 ❌

For more details on these failures, see this check.

Results for commit 78db3b5.

♻️ This comment has been updated with latest results.

ferdymercury · 2026-04-14T13:20:56Z

@bellenot could you let me know what is the contents (filenames) inside:

C:/libs/x64/fftw/3.3.5/lib on the Windows runners?

Or alternatively, what targets are defined once you run find_package with the new FindFFTW.cmake script in this branch ?

bellenot · 2026-04-14T14:06:53Z

@ferdymercury here is the content of C:\libs\x64\fftw\3.3.5\lib:

 Directory of C:\libs\x64\fftw\3.3.5\lib

10/11/2025  13:17    <DIR>          .
10/11/2025  13:17    <DIR>          ..
04/03/2022  11:47           238,966 fftw3-3.lib
03/03/2022  09:13           241,990 libfftw3-3.lib
03/03/2022  09:13           247,600 libfftw3f-3.lib
03/03/2022  09:13           149,230 libfftw3l-3.lib
               4 File(s)        877,786 bytes
               2 Dir(s)  73,924,689,920 bytes free

And I'll try your branch...

bellenot · 2026-04-14T14:17:27Z

@ferdymercury BTW, what is the problem with FFTW?

bellenot · 2026-04-14T14:23:51Z

And here is what CMake finds:

bellenot · 2026-04-14T14:56:13Z

Here are (some of) the errors on Windows:

[...]
    Exit code 0xc0000135
    Exit code 0xc0000135
    Exit code 0xc0000135
  C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Microsoft\VC\v180\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'C:\ROOT-CI\build\CMakeFiles\d062ca236c6013e32d1af865d69dc514\G__RooFitJSONInterface.cxx.rule;C:\ROOT-CI\src\roofit\jsoninterface\CMakeLists.txt' exited with code 1. [C:\ROOT-CI\build\roofit\jsoninterface\G__RooFitJSONInterface.vcxproj]
    Generating G__ROOTGpadv7.cxx, ../../bin/libROOTGpadv7_rdict.pcm, ../../bin/libROOTGpadv7.rootmap
  C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Microsoft\VC\v180\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'C:\ROOT-CI\build\CMakeFiles\9c31611484f5c242b181395c24233222\DataFrameSnapshotUtils.cxx.rule;C:\ROOT-CI\src\tree\dataframe\test\CMakeLists.txt' exited with code 1. [C:\ROOT-CI\build\tree\dataframe\test\DataFrameSnapshotUtils.vcxproj]
    Generating RXTupleDict.cxx, libntuple_compat_rdict.pcm, libntuple_compat.rootmap
  C:\Program Files\Microsoft Visual Studio\18\Community\MSBuild\Microsoft\VC\v180\Microsoft.CppCommon.targets(254,5): error MSB8066: Custom build for 'C:\ROOT-CI\build\CMakeFiles\4216675670bde90fccb32452aa24052b\G__XMLParser.cxx.rule;C:\ROOT-CI\src\io\xmlparser\CMakeLists.txt' exited with code 1. [C:\ROOT-CI\build\io\xmlparser\G__XMLParser.vcxproj]
    Exit code 0xc0000135
    Exit code 0xc0000135
[...]

Hard to tell where they are coming from (i.e, which DLL is not found). I'll have to debug it locally

ferdymercury · 2026-04-14T15:26:34Z

what is the problem with FFTW?

The current FindFFTW.cmake did not define a proper target so I borrowed a more up-to-date one by @egpbos but it seems it had a bug for Win32. I just fixed it so that it now finds all variants of FFTW3 correctly on Windows.

bellenot · 2026-04-15T07:01:52Z

@ferdymercury FYI the issue is coming from TBB. I'm looking into it

hageboeck

This is a good opportunity to make the builtins conform to what find modules would output. This means that we need to set the following variables (from here):

Xxx_INCLUDE_DIRS
Xxx_LIBRARIES

We also need to remember that:

They must not be CACHE variables
And they need to be set in the PARENT_SCOPE now that they are in subdirectories

I see that for some deps, this is done, for others it's not, so we have a chance now to unify this.

[cmake] try define proper gsl builtin target canonical names by findgsl [gsl] fix install path

ferdymercury · 2026-04-15T15:57:40Z

This is a good opportunity to make the builtins conform to what find modules would output. This means that we need to set the following variables (from here):
Xxx_INCLUDE_DIRS
Xxx_LIBRARIES
We also need to remember that:
* They must not be CACHE variables

* And they need to be set in the `PARENT_SCOPE` now that they are in subdirectories
I see that for some deps, this is done, for others it's not, so we have a chance now to unify this.

Thanks, good idea. I have fixed most now.

Additional fixes are in the corresponding PRs:
#21898
#21806
#21857
#21916

In any case, if/when all these are in, I was planning to do an additional uniformization formatting PR, meaning all builtins should start with the same lines:

# **PLEASE UPDATE ALSO THE FOLLOWING LINE WHEN UPDATING THE VERSION**
# 19 Feb 2024, https://whatever

and have all ROOT_PACKAGE_VERSION rather than packageVersion and things like that.

hageboeck

Thanks for the updates! I found a few more things to look at. Do we know the licence of the file that you copied?

hageboeck · 2026-04-16T11:31:19Z

+                -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
+                -DCMAKE_CXX_FLAGS=${GTEST_CXX_FLAGS}
+                -DCMAKE_AR=${CMAKE_AR}
+                -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}


If I'm not mistaken, this would try to install gtest in something like /usr/local. Instead of disabling the install command, once could make this:

${CMAKE_BINARY_DIR}/builtins/gtest

Suggested change

-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}

-DCMAKE_INSTALL_PREFIX=${CMAKE_BINARY_DIR}/builtins/googletest

Edit: I see that you only moved that code over. I think we can leave it for a later time to simplify this file a bit.

Yep, just moved. Shall I apply all / none / or a part of the commits you suggested on this file?

There is also VDT building in binary-dir rather than in builtins/ as the others but did not change that either. Was thinking to do the uniformization later on transversally for all once we get the overview of all.

hageboeck · 2026-04-16T11:34:45Z

+  set_target_properties(${lib} PROPERTIES
+    IMPORTED_LOCATION "${_G_LIBRARY_PATH}${CMAKE_STATIC_LIBRARY_PREFIX}${lib}${CMAKE_STATIC_LIBRARY_SUFFIX}"
+  )
+  add_dependencies(${lib} googletest)


Let's add the "canonical" names that are now used in ROOT:

Suggested change

add_dependencies(${lib} googletest)

add_dependencies(${lib} googletest)

add_library(GTest::${lib} ALIAS ${lib})

hageboeck · 2026-04-16T12:31:29Z

+# - Find the FFTW library
+# https://github.com/egpbos/findFFTW/
+# Original version of this file:
+#   Copyright (c) 2015, Wenzel Jakob
+#   https://github.com/wjakob/layerlab/blob/master/cmake/FindFFTW.cmake, commit 4d58bfdc28891b4f9373dfe46239dda5a0b561c6
+# Modifications:
+#   Copyright (c) 2017, Patrick Bos


@dpiparo, OK to take over other people's find files?

I think license-technically you should include the license text as well, which could also be done as a header in the FindFFTW.cmake file itself rather than as a separate file. But I won't press charges either way ;)

ferdymercury · 2026-04-16T12:44:13Z

Do we know the licence of the file that you copied?

FFTW: It's BSD-2 originally and later BSD-3

and misc improvements Co-authored-by: Stephan Hageboeck <stephan.hageboeck@cern.ch> Co-authored-by: ferdymercury <ferdymercury@users.noreply.github.com>

ferdymercury force-pushed the builtinsub branch 3 times, most recently from c5e599c to cb03ce1 Compare April 13, 2026 13:40

ferdymercury requested a review from guitargeek April 13, 2026 13:54

devajithvs assigned guitargeek Apr 13, 2026

ferdymercury added the pr:squash on merge label Apr 13, 2026